-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use offline registry library to generate minimum hive #1
Use offline registry library to generate minimum hive #1
Conversation
This change adds functions to generate valid, empty hives. Signed-off-by: Gabriel Adrian Samfira <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very nice. I've left some thoughts, and overall this is a vast improvement over the messing-about I was trying to do with libhivex.
Apologies for still not being mentally available for your work here, I'm still overwhelmed with post-move stuff and studies; I suspect I won't be able to bring full attention to bear here until mid-December at best.
internal/winapi/winapi.go
Outdated
@@ -1,3 +1,3 @@ | |||
package winapi | |||
|
|||
//go:generate go run ..\..\mksyscall_windows.go -output zsyscall_windows.go bindflt.go user.go console.go system.go net.go path.go thread.go jobobject.go logon.go memory.go process.go processor.go devices.go filesystem.go errors.go | |||
//go:generate go run ..\..\mksyscall_windows.go -output zsyscall_windows.go bindflt.go user.go console.go system.go net.go path.go thread.go jobobject.go logon.go memory.go process.go processor.go devices.go filesystem.go errors.go ofregistry.go |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest offreg.go
to match the DLL name. ofregistry.go
is a little odd.
return fmt.Errorf("accessing %s: %w", path, err) | ||
} | ||
|
||
version := windows.RtlGetVersion() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a chance for this to go wrong due to manifest shenanigans? Is there any risk in hard-coding 10.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hives we're generating use an old format. I think we could even use Windows XP as a version and still be fine. My understanding is that without manifesting, we default to Windows 8 which is fine I think. I can hard code 10.0 if you prefer.
internal/winapi/ofregistry.go
Outdated
//sys ORCreateHive(key *syscall.Handle) (regerrno error) = offreg.ORCreateHive | ||
//sys ORSaveHive(key syscall.Handle, file *uint16, OsMajorVersion uint32, OsMinorVersion uint32) (regerrno error) = offreg.ORSaveHive |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the overlap with moby/buildkit#3248, is it worth (at some point, not a blocker here) trying to add nice safe wrappers for offreg.dll to go-winio? (Although since my own wrappers for the volume-mounting APIs haven't landed, that's probably not a high-urgency activity).
If that's a direction we do want to go, I'd suggest structuring the code here to be ready to drop-in to go-wino, e.g., define the trivial wrapper functions (mostly bundling up syscall.UTF16PtrFromString
calls in my experience, and some light error handling on top of the auto-gen'd code) in a separate file or package.
See https://github.com/Microsoft/hcsshim/blob/main/cmd/wclayer/volumemountutils.go for how I went about this last time, and microsoft/go-winio#187 for the resulting go-winio PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And on that note, am I misremembering, or does mksyscall_windows.go
already know how to turn a string
into a *uint16
, e.g., see CMLocateDevNode
which is defined as
//sys CMLocateDevNode(pdnDevInst *uint32, pDeviceID string, uFlags uint32) (hr error) = cfgmgr32.CM_Locate_DevNodeW
and generates a wrapper func calling syscall.UTF16PtrFromString
for us.
(If this is the case, then there'd be no strong value in adding this to go-winio anyway, except for the benefit of callers that aren't already using mksyscall_windows
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at adding ofreg to go-winio. It may be useful in other projects.
Signed-off-by: Gabriel Adrian Samfira <[email protected]>
85a1d0b
to
3d6b81e
Compare
Thank you for delivering this all upstream. |
This change adds functions to generate valid, empty hives using the offline registry library (thanks @riverar!) and replaces
ensureFile()
.This should give you the same result as the code in this commit: 035362c
But without any keys at all, besides the root key.
Signed-off-by: Gabriel Adrian Samfira [email protected]